Improve consistency of exception message formatting. #3536

jwage · 2019-04-30T02:24:01Z

Q	A
Type	improvement
BC Break	no
Fixed issues	fixes #3531

Summary

Improve consistency of exception message formatting. See #3531 for details.

TODO

Add tests for exceptions that weren't tested.

lib/Doctrine/DBAL/DBALException.php

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php

lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php

lib/Doctrine/DBAL/Exception/MissingSQLParam.php

lib/Doctrine/DBAL/Id/TableGenerator.php

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php

lib/Doctrine/DBAL/Query/Exception/NonUniqueAlias.php

morozov · 2019-04-30T16:46:50Z

@jwage please see the comments above. I haven't reviewed the entire patch yet.

lib/Doctrine/DBAL/DBALException.php

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php

lib/Doctrine/DBAL/DBALException.php

lib/Doctrine/DBAL/Driver/PDOStatement.php

tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php

lib/Doctrine/DBAL/Exception/FormatArray.php

lib/Doctrine/DBAL/Schema/Table.php

lib/Doctrine/DBAL/Schema/UniqueConstraint.php

tests/Doctrine/Tests/DBAL/ConnectionTest.php

tests/Doctrine/Tests/DBAL/Exception/InvalidColumnIndexTest.php

tests/Doctrine/Tests/DBAL/Functional/PDOStatementTest.php

lib/Doctrine/DBAL/Exception/MissingSQLType.php

lib/Doctrine/DBAL/Exception/MissingArrayParameterType.php

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php

lib/Doctrine/DBAL/Driver/PDOStatement.php

lib/Doctrine/DBAL/Types/Exception/ValueNotConvertible.php

lib/Doctrine/DBAL/Types/Exception/SerializationFailed.php

lib/Doctrine/DBAL/Exception/FormatVariable.php

lib/Doctrine/DBAL/DBALException.php

jwage · 2019-05-01T19:40:20Z

lib/Doctrine/DBAL/Cache/ArrayStatement.php

    public function closeCursor() : void
    {
-        unset($this->data);
+        $this->data = null;


I was adding tests for ArrayStatement and this seemed wrong since it can result in PHP errors if you call other public methods after closeCursor()

Makes perfect sense.

lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php

jwage · 2019-05-01T19:51:26Z

lib/Doctrine/DBAL/Sharding/PoolingShardConnection.php


            if (isset($this->connectionParameters[$shard['id']])) {
-                throw new InvalidArgumentException('Shard ' . $shard['id'] . ' is duplicated in the configuration.');
+                throw new InvalidArgumentException(sprintf('Shard "%s" is duplicated in the configuration.', $shard['id']));


Is $shard['id'] a string?

It's annotated as int|string in most places but looks like it's a numeric string (or an integer):

dbal/lib/Doctrine/DBAL/Sharding/PoolingShardConnection.php

Lines 94 to 96 in 82f5fe3

if (! is_numeric($shard['id']) || $shard['id'] < 1) {

throw new InvalidArgumentException('Shard Id has to be a non-negative number.');

}

Also, the id comes from sys.federation_member_distributions.member_id:

dbal/lib/Doctrine/DBAL/Sharding/SQLAzure/SQLAzureShardManager.php

Lines 158 to 164 in 68e48e8

$sql = 'SELECT member_id as id,

distribution_name as distribution_key,

CAST(range_low AS CHAR) AS rangeLow,

CAST(range_high AS CHAR) AS rangeHigh

FROM sys.federation_member_distributions d

INNER JOIN sys.federations f ON f.federation_id = d.federation_id

WHERE f.name = ' . $this->conn->quote($this->federationName);

which according to this article is INT.

These details are mostly for myself, I'll need this info later to finish the work on strict types.

morozov · 2019-05-06T19:54:18Z

@jwage I rebased develop yesterday to accommodate recent conflicting changes from master. Please rebase.

jwage · 2019-05-07T01:11:48Z

@morozov Done!

morozov · 2019-05-07T18:30:06Z

lib/Doctrine/DBAL/Exception/InvalidPlatformType.php

        return new self(
            sprintf(
-                "Option 'platform' must be an object and subtype of '%s'. Got '%s'",
+                'Option "platform" must be an object and subtype of %s. Got "%s".',


No quotes around type needed.

morozov · 2019-05-07T18:30:24Z

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php


-        throw new UnexpectedValueException("Unrecognized boolean literal '${value}'");
+        throw new UnexpectedValueException(sprintf(
+            'Unrecognized boolean literal, "%s" given.',


No quotes around type needed. Or… most likely it will be a printable scalar value. Let's leave it without the GetVariableType.

morozov · 2019-05-07T18:35:15Z

@jwage thanks. Could you look at the build failure and the new comments? Looks like we're really close to finishing it.

jwage · 2019-05-07T21:30:34Z

Fixed latest feedback and fixed the tests.

morozov · 2019-05-08T03:01:30Z

Thank you 🚢

jwage · 2019-05-08T03:20:20Z

Cool! Thanks!

Improve consistency of exception message formatting.

jwage added WIP Improvement labels Apr 30, 2019

jwage added this to the 3.0.0 milestone Apr 30, 2019

jwage marked this pull request as ready for review April 30, 2019 02:25

jwage requested a review from morozov April 30, 2019 03:19

greg0ire reviewed Apr 30, 2019

View reviewed changes

lib/Doctrine/DBAL/DBALException.php Outdated Show resolved Hide resolved

morozov reviewed Apr 30, 2019

View reviewed changes

jwage commented Apr 30, 2019

View reviewed changes

lib/Doctrine/DBAL/DBALException.php Outdated Show resolved Hide resolved

jwage commented Apr 30, 2019

View reviewed changes

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php Outdated Show resolved Hide resolved

greg0ire reviewed Apr 30, 2019

View reviewed changes

lib/Doctrine/DBAL/DBALException.php Outdated Show resolved Hide resolved

morozov reviewed Apr 30, 2019

View reviewed changes

morozov reviewed May 1, 2019

View reviewed changes

lib/Doctrine/DBAL/Exception/MissingSQLType.php Outdated Show resolved Hide resolved

morozov reviewed May 1, 2019

View reviewed changes

jwage commented May 1, 2019

View reviewed changes

lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php Outdated Show resolved Hide resolved

jwage commented May 1, 2019

View reviewed changes

morozov force-pushed the develop branch from e46c09b to 5ee9443 Compare May 6, 2019 04:48

morozov reviewed May 7, 2019

View reviewed changes

Improve consistency of exception message formatting.

67ebcf6

morozov approved these changes May 8, 2019

View reviewed changes

morozov merged commit c7cb259 into doctrine:develop May 8, 2019

morozov self-assigned this May 8, 2019

morozov added a commit that referenced this pull request May 23, 2019

Merge pull request #3536 from jwage/improve-exception-messages

89766f8

Improve consistency of exception message formatting.

morozov removed the WIP label May 23, 2019

jwage added the Error Handling label May 30, 2019

morozov added a commit to morozov/dbal that referenced this pull request May 31, 2019

Merge pull request doctrine#3536 from jwage/improve-exception-messages

e9379e4

Improve consistency of exception message formatting.

morozov added a commit to morozov/dbal that referenced this pull request May 31, 2019

Merge pull request doctrine#3536 from jwage/improve-exception-messages

049fa7d

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Jun 13, 2019

Merge pull request #3536 from jwage/improve-exception-messages

aac30ec

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Jun 27, 2019

Merge pull request #3536 from jwage/improve-exception-messages

b6ab8df

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Jun 27, 2019

Merge pull request #3536 from jwage/improve-exception-messages

a91b18c

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Jun 27, 2019

Merge pull request #3536 from jwage/improve-exception-messages

6f209f7

Improve consistency of exception message formatting.

morozov added a commit to morozov/dbal that referenced this pull request Aug 26, 2019

Merge pull request doctrine#3536 from jwage/improve-exception-messages

3e782c7

Improve consistency of exception message formatting.

morozov added a commit to morozov/dbal that referenced this pull request Aug 27, 2019

Merge pull request doctrine#3536 from jwage/improve-exception-messages

c36f826

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Nov 2, 2019

Merge pull request #3536 from jwage/improve-exception-messages

5193c97

Improve consistency of exception message formatting.

morozov mentioned this pull request Apr 17, 2020

allow using multiple connections for CLI commands #3956

Merged

morozov mentioned this pull request May 4, 2020

Segregated support of unique index and unique constraint #3980

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2022

	if (! is_numeric($shard['id']) \|\| $shard['id'] < 1) {
	throw new InvalidArgumentException('Shard Id has to be a non-negative number.');
	}

	$sql = 'SELECT member_id as id,
	distribution_name as distribution_key,
	CAST(range_low AS CHAR) AS rangeLow,
	CAST(range_high AS CHAR) AS rangeHigh
	FROM sys.federation_member_distributions d
	INNER JOIN sys.federations f ON f.federation_id = d.federation_id
	WHERE f.name = ' . $this->conn->quote($this->federationName);

Uh oh!

Improve consistency of exception message formatting. #3536

Improve consistency of exception message formatting. #3536

Uh oh!

Conversation

jwage commented Apr 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

TODO

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

morozov commented Apr 30, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jwage May 1, 2019

Choose a reason for hiding this comment

Uh oh!

morozov May 3, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jwage May 1, 2019

Choose a reason for hiding this comment

Uh oh!

morozov May 1, 2019

Choose a reason for hiding this comment

Uh oh!

morozov commented May 6, 2019

Uh oh!

jwage commented May 7, 2019

Uh oh!

morozov May 7, 2019

Choose a reason for hiding this comment

Uh oh!

morozov May 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morozov commented May 7, 2019

Uh oh!

jwage commented May 7, 2019

Uh oh!

morozov commented May 8, 2019

Uh oh!

jwage commented May 8, 2019

Uh oh!

Reviewers

jwage commented Apr 30, 2019 •

edited

Loading

morozov May 7, 2019 •

edited

Loading